Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed to show spinner on data load #1491

Merged
merged 2 commits into from Oct 10, 2018
Merged

Conversation

stephanwlee
Copy link
Contributor

Also fixed the issue where, when showing the spinner, the iron-collapse
does not always open. Note that this collapse bug did exist in version
1,10 too,

Also fixed the issue where, when showing the spinner, the iron-collapse
does not always open. Note that this collapse bug did exist in version
1,10 too,
@@ -120,7 +119,8 @@ export const DataLoaderBehavior = {

if (!this.isAttached) return;
this._loadDataAsync = this.async(() => {
this.dataLoading = true;
// Using a setter does not notify the impl observers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is basically the same issue as the loadKey issue, right? So it seems like maybe the behavior observer/property connection is buggy or broken somehow.

@@ -140,7 +140,7 @@ export const DataLoaderBehavior = {
});

return Promise.all(promises).then(this._canceller.cancellable(result => {
this.dataLoading = false;
this.set('dataLoading', false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the same comment here?

@@ -58,7 +58,6 @@ export const DataLoaderBehavior = {

dataLoading: {
type: Boolean,
readOnly: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to be non-readOnly in order to use set()? It's still logically readonly though, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you were correct! I always forget that you ned to use a special setter when setting a value to a readOnly property. I changed other codes to use this._setDataLoading and kept this readOnly. Thanks!

@stephanwlee stephanwlee merged commit 44c830f into tensorflow:master Oct 10, 2018
@stephanwlee stephanwlee deleted the spin branch October 10, 2018 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants